Hey @aeworxet If we leave the bundler out of the picture here, can you add a an example on which the optimizer is not producing the right output and how this PR resolves the issue?

Right now, Optimizer produces output from the left side of the diff. The code in the PR adds components/ to make the $ref correct (right side of the diff.)

My main question is whether $ref in line 29 should be left intact or should it also be added the components/, because both versions are correct.

It's just that in the current version, the channel component in lines 6-8 has meaning because, with it, the AsyncAPI Document retains readability for humans in general. And if even one $ref stops referring to it, there will not be even one reason it's still there (it's not mandatory to have #/channels according to the Spec.)

@aeworxet based on the specification of Operation Object in v3:

If the operation is located in the root Operations Object, it MUST point to a channel definition located in the root Channels Object, and MUST NOT point to a channel definition located in the Components Object or anywhere else. If the operation is located in the Components Object, it MAY point to a Channel Object in any location.

and for messages:

It MUST contain a subset of the messages defined in the channel referenced in this operation, and MUST NOT point to a subset of message definitions located in the Messages Object in the Components Object or anywhere else.

I think the issue is bigger.

1

If the operation is located in the root Operations Object, it MUST point to a channel definition located in the root Channels Object, and MUST NOT point to a channel definition located in the Components Object or anywhere else.

Operation Object in lines 9-11 doesn't point to a channel. It points to a copy of itself in the Components Object, which is either OK or not allowed at all.
It passed Parser's validation, so it's OK.
Spec doesn't mention such a possibility, so it's not OK.

Which one is it?

@derberg @jonaslagoni @smoya

2

If the operation is located in the Components Object, it MAY point to a Channel Object in any location.

So it is allowed to move Operation Object to Components Object, but the Operation Object is not supposed to be in the root of the AsyncAPI Document after moveAllToComponents() then, as I understand. 

Or should it be there in the root, but pointing only to channels in the root? And if channels are deleted from root after moving? Should the root Operations Object be tied to the root Channels Object and either they are existing both in the root or none of them do?

And not only the fact that the root contains Operation Object pointing to a copy of itself in Components Object, but also the fact that it contains an invalid $ref (pointing to operations which is not mentioned in Spec) passed Parser's validation.

3

Optimizer's output.yaml didn't pass separate Parser's validation with asyncapi validate, but then it wasn't supposed to be given out by Optimizer as an output file to begin with.

$ asyncapi validate output.yaml 

File output.yaml and/or referenced documents have governance issues.

Errors 
output.yaml
 31:11  error  asyncapi3-operation-messages-from-referred-channel  Operation message does not belong to the specified channel.  components.operations.onUserSignUp.messages[0]

And when I remove components/ from the 31:11 $ref to make it incorrect indeed, the file (which now has an intentionally wrong $ref) passes validation...

So I think that Parser's validation rules need to be reviewed, and there needs to be implemented Parser's pre-validation, which will validate optimized files before giving them outside, as I did it with Bundler.

Comments?

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Implemented pre-validation of the optimized AsyncAPI Documents before their output. Of course, some tests immediately started failing (which is good, because this means validation works.)
Fixing the mechanism of testing.

Checked outputs manually and here are some remarks.

@asyncapi/parser v3.0.14

• input inputYAML / inputJSON
• output *_mATCTrue_mDTCFalse_schemaFalse, *_mATCTrue_mDTCFalse_schemaTrue

Input file validates OK.

Output file gives validation errors:

[
  {
    code: 'asyncapi-document-resolved',
    message: '"user~1deleteAccount" property must have required property "action"',
    path: [ 'operations', 'user/deleteAccount' ],
    severity: 0,
    range: { start: [Object], end: [Object] }

    // there is no `user~1deleteAccount` property, there is a `user/deleteAccount.subscribe` property
    // that has child property `action` in both input and output YAMLs,
    // and there is a `user/deleteAccount` which DOES NOT have the child property `action`.
    // which one does Parser validate?

  },
  {
    code: 'asyncapi-document-resolved',
    message: 'Property "subscribe" is not expected to be here',
    path: [ 'operations', 'user/deleteAccount', 'subscribe' ],
    severity: 0,
    source: undefined,
    range: { start: [Object], end: [Object] }

    // output file contains
                                operations:
                                  user/deleteAccount.subscribe:
                                    action: send
                                    channel:
                                      $ref: '#/channels/deleteAccount'
                                    messages:
                                      - $ref: '#/channels/deleteAccount/messages/deleteUser'
                                  user/deleteAccount:
                                    subscribe:
                                      $ref: '#/components/operations/subscribe'

    // so property "subscribe" is expected or not expected to be here?

    // double parsing is on me because it should not have happened,
    // the `dot` was parsed incorrectly during optimization

    // Parser validates `user/deleteAccount.subscribe` OK in the input file.

  },
  {
    code: 'invalid-ref',
    path: [ 'operations', 'user/deleteAccount', 'subscribe', '$ref' ],
    message: "'#/components/operations/subscribe' does not exist",
    severity: 0,
    range: { start: [Object], end: [Object] }
  }
  
    // this is obviously about
                                  user/deleteAccount:
                                    subscribe:
                                      $ref: '#/components/operations/subscribe'
    // this on is on me too because $ref IS wrong

]

There is no limitation on the usage of . (dot) for naming properties in an AsyncAPI Document, and YAML containing the property user/deleteAccount.subscribe passes validation with Parser.

What mask should I use to find property

user/deleteAccount.subscribe

in this file, then?

{
  path: 'channels.withFullFormMessage.messages.canBeReused',
  action: 'move',
  target: 'components.messages.canBeReused'
},
{
  path: 'channels.withDuplicatedMessage1.messages.duped1',
  action: 'move',
  target: 'components.messages.duped1'
},
{
  path: 'channels.withDuplicatedMessage2.messages.duped2',
  action: 'move',
  target: 'components.messages.duped2'
},
{
  path: 'operations.user/deleteAccount.subscribe', // <-- I need to find this 'user/deleteAccount.subscribe'
  action: 'move',
  target: 'components.operations.subscribe'
},
{
  path: 'channels.withDuplicatedMessage1',
  action: 'move',
  target: 'components.channels.withDuplicatedMessage1FromXOrigin'
},
{
  path: 'channels.withDuplicatedMessage2',
  action: 'move',
  target: 'components.channels.withDuplicatedMessage2'
},

The issues raised are too complex and should be resolved through separate PRs, so I'm closing this one.